-
Notifications
You must be signed in to change notification settings - Fork 248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updates beacon agent on-prem DSN examples #6213
base: develop
Are you sure you want to change the base?
Conversation
932d3e4
to
6a7d76b
Compare
advocacy_docs/edb-postgres-ai/console/estate/agent/install-agent.mdx
Outdated
Show resolved
Hide resolved
@@ -158,8 +158,7 @@ agent: | |||
provider: | |||
onprem: | |||
databases: | |||
sales_reporting: | |||
dsn: "$DSN" | |||
- dsn: "$DSN" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In https://github.com/EnterpriseDB/docs/pull/6105/files a resource_id key is used; are we skipping it here for compactness?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource id is an allowed field according to the code https://github.com/EnterpriseDB/upm-beacon/blob/79f12896839dd5c8898c97c4ef2591645de57a99/internal/config/agent.go#L211
It is an optional field though, and doesn't need to be specified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR looks totally valid https://github.com/EnterpriseDB/docs/pull/6105/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, yes but it also conflicts with your edits which is what I want to resolve. Given the edits in here, I'd like to promote this PR and ensure nothing is lost from the other PR before we close it.
advocacy_docs/edb-postgres-ai/console/estate/agent/install-agent.mdx
Outdated
Show resolved
Hide resolved
tags: | ||
- "<tag-one>" | ||
- "<tag-two>" | ||
- dsn: "$DSN1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above - we have another PR where resource_id is used. The other option is that resource_id has been dropped, so in that case we should have a formal reference for the yaml in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this clashes with another PR, some concerns over what is definitive. Probably best resolved with a YAML format listing for completeness.
6d1cef9
to
da8c401
Compare
I think this PR can be closed based on our conversation. |
Happy to review and merge this one (as it has other fixes). We can doc resource_id properly elsewhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - will hold till it is confirmed released.
d5913f4
to
e7f92a4
Compare
e7f92a4
to
01e919d
Compare
What Changed?
These changes are aligned with upcoming product capabilities.
I've also eliminated trailing whitespaces in the touched docs.